Skip to content

lib: use more global primordials#35397

Closed
targos wants to merge 5 commits intonodejs:masterfrom
targos:fix-pr-ahkrin
Closed

lib: use more global primordials#35397
targos wants to merge 5 commits intonodejs:masterfrom
targos:fix-pr-ahkrin

Conversation

@targos
Copy link
Member

@targos targos commented Sep 28, 2020

This is a PR with all the changes of #31209, #31207, #31206 and #31203, after fixing conflicts with the permission of @Sebastien-Ahkrin.

@targos targos requested review from a team as code owners September 28, 2020 14:22
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 28, 2020

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/http2
  • @nodejs/modules
  • @nodejs/net
  • @nodejs/wasi

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 28, 2020
This was referenced Sep 28, 2020
Closed
Closed
Closed
Closed
ljharb
ljharb reviewed Sep 28, 2020
View reviewed changes
targos
targos commented Oct 2, 2020
View reviewed changes
lib/tls.js
Comment on lines +151 to +154
return StringPrototypeSplit(
StringPrototypeReplace(unfqdn(host), /[A-Z]/g, toLowerCase),
'.'
);
Copy link
Member Author

@targos targos Oct 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb @Trott I made the change, but I don't really like how the code looks like (that's why I'm currently only pushing for use of static primordials)...

Copy link
Member

@ljharb ljharb Oct 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance method primordials are used often in the node codebase; imo "how it looks" is only relevant once robustness against user code is assured.

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 2, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Oct 2, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/33324/

targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace Float64Array global with primordials
34b7b47 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace Int32Array global with primordials
6883c8d 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace Int8Array global with primordials
693cc2c 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
targos pushed a commit that referenced this pull request Oct 4, 2020
@Sebastien-Ahkrin @targos
lib: replace String global with primordials
b79829c 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
@targos
Copy link
Member Author

targos commented Oct 4, 2020

Landed in b8f34b2...b79829c

@targos targos closed this Oct 4, 2020
@targos targos deleted the fix-pr-ahkrin branch October 4, 2020 07:45
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace Float64Array global with primordials
f544f7a 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace Int32Array global with primordials
d6ba4ec 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace Int8Array global with primordials
ebf3900 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
BethGriggs pushed a commit that referenced this pull request Oct 13, 2020
@Sebastien-Ahkrin @BethGriggs
lib: replace String global with primordials
30c6b3e 
PR-URL: #35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
@MylesBorins MylesBorins mentioned this pull request Oct 14, 2020
Merged
@Leko Leko mentioned this pull request Oct 29, 2020
Closed
3 tasks
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace Float64Array global with primordials
690844c 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace Int32Array global with primordials
b896666 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace Int8Array global with primordials
146ea15 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@Sebastien-Ahkrin @joesepi
lib: replace String global with primordials
50f183f 
PR-URL: nodejs#35397
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Pranshu Srivastava <rexagod@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments

@Trott Trott Trott approved these changes

@lpinca lpinca lpinca approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

+3 more reviewers

@antsmartian antsmartian antsmartian approved these changes

@ryzokuken ryzokuken ryzokuken approved these changes

@rexagod rexagod rexagod approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@targos @nodejs-github-bot @ljharb @Trott @antsmartian @lpinca @gengjiawen @ryzokuken @rexagod @Sebastien-Ahkrin